fix: group chat SSH synthesis wrapping + structured logging#539
Open
openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
Open
fix: group chat SSH synthesis wrapping + structured logging#539openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds comprehensive structured logging to the group-chat-router module, replacing console.log statements with logger.debug calls and contextual metadata, while also introducing test cases that validate SSH wrapping behavior in moderator synthesis functions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR closes two infrastructure gaps in the group chat router: SSH wrapping is added to Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[spawnModeratorSynthesis called] --> B[Load chat & verify moderator active]
B --> C[Resolve agent config & build synthesisPrompt]
C --> D{sshStore set AND\nchat.moderatorConfig.sshRemoteConfig?}
D -- Yes --> E[wrapSpawnWithSsh\ncommand/args/cwd/prompt/envVars]
E --> F[spawnCommand = ssh\nspawnArgs = SSH args\nspawnCwdResolved = remote cwd]
D -- No --> G[spawnCommand = original\nspawnArgs = finalArgs\nspawnCwdResolved = os.homedir]
F --> H[getWindowsSpawnConfig\nfor shell/stdin config]
G --> H
H --> I[processManager.spawn\nwith resolved command/args/cwd/prompt/envVars]
I --> J[Moderator synthesis runs\nremotely or locally]
J --> K[Exit triggers routeModeratorResponse\nfor final answer or follow-up mentions]
Last reviewed commit: 0a3bc21 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+883
to
+886
| it('spawnModeratorSynthesis applies SSH wrapping when moderator has SSH config', async () => { | ||
| // Create a chat with SSH-enabled moderator config | ||
| const chat = await createTestChat('SSH Synthesis Test', 'claude-code'); | ||
| const sshModeratorConfig = { |
There was a problem hiding this comment.
Unused chat variable leaks an uncleaned test resource
createTestChat('SSH Synthesis Test', 'claude-code') on line 885 pushes its ID into createdChats (via the helper), creating a real on-disk chat object that is never referenced again. The test immediately creates a second chat (chatWithSsh) which is the one actually exercised. The first chat is dead code and should be removed.
Suggested change
| it('spawnModeratorSynthesis applies SSH wrapping when moderator has SSH config', async () => { | |
| // Create a chat with SSH-enabled moderator config | |
| const chat = await createTestChat('SSH Synthesis Test', 'claude-code'); | |
| const sshModeratorConfig = { | |
| it('spawnModeratorSynthesis applies SSH wrapping when moderator has SSH config', async () => { | |
| const sshModeratorConfig = { | |
| sshRemoteConfig: { | |
| enabled: true, | |
| remoteId: 'remote-1', | |
| workingDirOverride: '/home/user/project', | |
| }, | |
| }; | |
| // Update the chat to have moderator config with SSH | |
| // We need to create the chat with moderator config, then spawn moderator | |
| const chatWithSsh = await createGroupChat( | |
| 'SSH Synthesis Test 2', | |
| 'claude-code', | |
| sshModeratorConfig | |
| ); |
Comment on lines
270
to
274
| let chat = await loadGroupChat(groupChatId); | ||
| if (!chat) { | ||
| console.log(`[GroupChat:Debug] ERROR: Group chat not found!`); | ||
| logger.debug('Group chat not found', LOG_CONTEXT, { groupChatId }); | ||
| throw new Error(`Group chat not found: ${groupChatId}`); | ||
| } |
There was a problem hiding this comment.
Error-path conditions silently downgraded to debug level
Several branches that immediately throw are now logged only at logger.debug. In a production deployment with log_level=info or above — precisely the level intended for production — these will produce zero log output before the exception propagates. This makes post-mortem debugging significantly harder.
The same PR's spawnModeratorSynthesis function consistently uses logger.error for analogous conditions (lines 1154, 1164, 1200), so there is already an established pattern in this file:
| Location | Condition | Current level | Recommended |
|---|---|---|---|
routeUserMessage L272 |
Group chat not found (throws) | debug |
error |
routeUserMessage L283 |
Moderator not active (throws) | debug |
error |
routeUserMessage L406 |
Agent not available (throws) | debug |
error |
routeModeratorResponse L630 |
Group chat not found (throws) | debug |
error |
routeAgentResponse L1043 |
Group chat not found (throws) | debug |
error |
routeAgentResponse L1050 |
Participant not found (throws) | debug |
error |
The intent to gate verbose flow-tracing behind debug is sound, but branches that indicate something is definitively wrong and cause an exception should use at least logger.warn (recoverable) or logger.error (non-recoverable), consistent with spawnModeratorSynthesis.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/__tests__/main/group-chat/group-chat-router.test.ts (1)
783-792:⚠️ Potential issue | 🟡 MinorMake the SSH wrapper mock match the real return contract.
The real
wrapSpawnWithSsh()returns the local home directory ascwdand clears local env vars when SSH is used. This mock currently returns the remote working dir and{}, so it won't catch callers that ignore the wrapper's returnedcwd/customEnvVars.Suggested fix
beforeEach(() => { // Configure the SSH wrapping mock to return transformed spawn config mockWrapSpawnWithSsh.mockResolvedValue({ command: 'ssh', args: ['-t', 'user@pedtome.local', 'claude', '--print'], - cwd: '/home/user/project', + cwd: os.homedir(), prompt: 'test prompt', - customEnvVars: {}, + customEnvVars: undefined, sshRemoteUsed: { name: 'PedTome' }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/group-chat/group-chat-router.test.ts` around lines 783 - 792, The mock for mockWrapSpawnWithSsh doesn't match wrapSpawnWithSsh's real contract: when SSH is used its returned object sets cwd to the local home directory and provides cleared/removed local env vars via customEnvVars; update the mock in the test so mockWrapSpawnWithSsh.mockResolvedValue returns cwd pointing to the local home directory (e.g., the test's homedir) and customEnvVars reflecting cleared local env (e.g., an object that removes or omits local env keys) while still including sshRemoteUsed and the command/args so callers that rely on the wrapper's cwd/customEnvVars behavior are exercised.
🧹 Nitpick comments (1)
src/main/group-chat/group-chat-router.ts (1)
1276-1313: Extract the moderator SSH spawn prep into a shared helper.This is now effectively a second copy of the moderator spawn setup from
routeUserMessage(). Keeping both paths aligned already needed thesynthCwdfollow-up in this PR; centralizing the SSH/spawn preparation would make future SSH or prompt-handling changes much less likely to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 1276 - 1313, Extract the duplicated SSH-aware moderator spawn preparation into a single reusable helper (e.g., prepareModeratorSpawn or prepareSpawnWithSshWrapper) that encapsulates the logic currently duplicated in routeUserMessage() and this block in group-chat-router.ts: it should accept inputs like command, finalArgs, synthesisPrompt, agent (agent.promptArgs, agent.noPromptSeparator, agent.binaryName), configResolution.effectiveCustomEnvVars/getCustomEnvVarsCallback and chat.moderatorConfig.sshRemoteConfig/sshStore, call wrapSpawnWithSsh when appropriate, and return the resolved spawnCommand, spawnArgs, spawnCwdResolved, spawnPrompt and spawnEnvVars (and sshWrapped.sshRemoteUsed info for logging); then replace both original code blocks to call this helper and use its returned values, preserving the existing logger.debug calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/main/group-chat/group-chat-router.test.ts`:
- Around line 931-943: Add a positive assertion that a local spawn occurred so
the test fails if spawnModeratorSynthesis returned early: after
mockWrapSpawnWithSsh.mockClear(), also clear and/or reset the spawn mock on
mockProcessManager (e.g., mockProcessManager.spawn.mockClear()) and then after
calling spawnModeratorSynthesis assert that mockProcessManager.spawn was called
(e.g., expect(mockProcessManager.spawn).toHaveBeenCalled()). This uses the
existing symbols spawnModeratorSynthesis, mockWrapSpawnWithSsh, and
mockProcessManager.spawn to ensure we test the non-SSH local spawn path.
---
Outside diff comments:
In `@src/__tests__/main/group-chat/group-chat-router.test.ts`:
- Around line 783-792: The mock for mockWrapSpawnWithSsh doesn't match
wrapSpawnWithSsh's real contract: when SSH is used its returned object sets cwd
to the local home directory and provides cleared/removed local env vars via
customEnvVars; update the mock in the test so
mockWrapSpawnWithSsh.mockResolvedValue returns cwd pointing to the local home
directory (e.g., the test's homedir) and customEnvVars reflecting cleared local
env (e.g., an object that removes or omits local env keys) while still including
sshRemoteUsed and the command/args so callers that rely on the wrapper's
cwd/customEnvVars behavior are exercised.
---
Nitpick comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 1276-1313: Extract the duplicated SSH-aware moderator spawn
preparation into a single reusable helper (e.g., prepareModeratorSpawn or
prepareSpawnWithSshWrapper) that encapsulates the logic currently duplicated in
routeUserMessage() and this block in group-chat-router.ts: it should accept
inputs like command, finalArgs, synthesisPrompt, agent (agent.promptArgs,
agent.noPromptSeparator, agent.binaryName),
configResolution.effectiveCustomEnvVars/getCustomEnvVarsCallback and
chat.moderatorConfig.sshRemoteConfig/sshStore, call wrapSpawnWithSsh when
appropriate, and return the resolved spawnCommand, spawnArgs, spawnCwdResolved,
spawnPrompt and spawnEnvVars (and sshWrapped.sshRemoteUsed info for logging);
then replace both original code blocks to call this helper and use its returned
values, preserving the existing logger.debug calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7738f9fc-9f97-4fc8-9ffa-c504103f7c17
📒 Files selected for processing (2)
src/__tests__/main/group-chat/group-chat-router.test.tssrc/main/group-chat/group-chat-router.ts
Comment on lines
+931
to
+943
| it('spawnModeratorSynthesis does NOT apply SSH wrapping when no SSH config', async () => { | ||
| // Create a chat without SSH config | ||
| const chat = await createTestChatWithModerator('No SSH Synthesis Test'); | ||
| await addParticipant(chat.id, 'Worker', 'claude-code', mockProcessManager); | ||
|
|
||
| setSshStore(mockSshStore); | ||
| mockWrapSpawnWithSsh.mockClear(); | ||
|
|
||
| await spawnModeratorSynthesis(chat.id, mockProcessManager, mockAgentDetector); | ||
|
|
||
| // SSH wrapping should NOT be called since chat has no moderatorConfig.sshRemoteConfig | ||
| expect(mockWrapSpawnWithSsh).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Add a positive assertion that synthesis still spawned locally.
This test passes if SSH wrapping is skipped, but it also passes if spawnModeratorSynthesis() returns early before the spawn path. Clearing the prior spawn calls and asserting a non-SSH spawn would make the regression signal much tighter.
Suggested fix
it('spawnModeratorSynthesis does NOT apply SSH wrapping when no SSH config', async () => {
// Create a chat without SSH config
const chat = await createTestChatWithModerator('No SSH Synthesis Test');
await addParticipant(chat.id, 'Worker', 'claude-code', mockProcessManager);
setSshStore(mockSshStore);
mockWrapSpawnWithSsh.mockClear();
+ mockProcessManager.spawn.mockClear();
await spawnModeratorSynthesis(chat.id, mockProcessManager, mockAgentDetector);
// SSH wrapping should NOT be called since chat has no moderatorConfig.sshRemoteConfig
expect(mockWrapSpawnWithSsh).not.toHaveBeenCalled();
+ expect(mockProcessManager.spawn).toHaveBeenCalled();
+ const [spawnConfig] = mockProcessManager.spawn.mock.calls[0];
+ expect(spawnConfig.command).not.toBe('ssh');
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/group-chat/group-chat-router.test.ts` around lines 931 -
943, Add a positive assertion that a local spawn occurred so the test fails if
spawnModeratorSynthesis returned early: after mockWrapSpawnWithSsh.mockClear(),
also clear and/or reset the spawn mock on mockProcessManager (e.g.,
mockProcessManager.spawn.mockClear()) and then after calling
spawnModeratorSynthesis assert that mockProcessManager.spawn was called (e.g.,
expect(mockProcessManager.spawn).toHaveBeenCalled()). This uses the existing
symbols spawnModeratorSynthesis, mockWrapSpawnWithSsh, and
mockProcessManager.spawn to ensure we test the non-SSH local spawn path.
Introduce maestro:// URL scheme support for navigating to agents, tabs, and groups from external apps and OS notification clicks. - Add deep-links module with URL parsing, protocol registration, single-instance locking, and cross-platform event handling - Wire notification click handlers to navigate to the originating agent/tab via deep link dispatch - Thread sessionId/tabId context through notification preload bridge - Add onDeepLink listener in renderer with routing to existing navigation handlers - Register maestro:// protocol in electron-builder config - Add 18 tests covering URL parsing and notification click wiring
- URI-encode sessionId/tabId when constructing deep link URLs in notification click handler to prevent malformed URLs with special chars - Add process.exit(0) after app.quit() so secondary instances exit immediately without running further module-level setup - Use useRef for sessions in deep link effect to avoid tearing down and re-registering the IPC listener on every sessions change - Guard against navigating to non-existent session IDs in deep link handler to prevent invalid UI state - Add cross-reference comment in global.d.ts linking to canonical ParsedDeepLink type (can't import in ambient declaration file) - Add test for URI-encoding round-trip in notification click handler
- Add shared deep-link-urls.ts with buildSessionDeepLink(),
buildGroupDeepLink(), and buildFocusDeepLink() utilities
- Add {{AGENT_DEEP_LINK}}, {{TAB_DEEP_LINK}}, {{GROUP_DEEP_LINK}}
template variables available in system prompts, custom AI commands,
and Auto Run documents
- Wire activeTabId and groupId into TemplateContext at all call sites
(agentStore, useInputProcessing, useRemoteHandlers,
useDocumentProcessor, useMergeTransferHandlers, batch-processor)
- Refactor notifications.ts to use shared buildSessionDeepLink()
- Add sessionId/tabId to notifyToast callers where context is available
(merge, transfer, summarize, PR creation)
- Add docs/deep-links.md documentation page with URL format, usage
examples, template variables, and platform behavior
- Add 8 tests for URL builders, 6 tests for template variable
substitution including URI encoding
…e activity indicators Broadcast new history entries via IPC when they are added, subscribe in the UnifiedHistoryTab with RAF batching and deduplication, and extend the HistoryStatsBar with spinning Active agent count and Queued message count indicators derived from the Zustand session store.
…orrect types - Replace unstable sessionNameMap Zustand selector (new Map per render) with a stable ref + subscribe pattern to avoid streaming effect re-subscription - Dedupe within batch before merging; compute setTotalEntries and setHistoryStats from deduplicated entries only (not raw batch) - Clear pendingEntriesRef on cleanup to prevent stale replay after resubscribe - Use HistoryEntry (not UnifiedHistoryEntry) in preload callback type since the wire payload lacks sourceSessionId - Use canonical UsageStats interface in global.d.ts (fixes pre-existing cacheReadTokens/cacheWriteTokens field name mismatch)
…URL registry support - Add `symphony: boolean` (default true) to EncoreFeatureFlags - Gate Symphony modal, menu item, keyboard shortcut (⇧⌘Y), and command palette entry - Add `symphonyRegistryUrls` setting for user-configured additional registry URLs - Replace single `fetchRegistry()` with `fetchRegistries()` that fetches default + custom URLs in parallel - Merge repositories by slug (default registry wins on conflicts), isolated per-URL error handling - Add Symphony toggle + Registry Sources UI in Settings > Encore tab - Update tests for new symphony flag across all encore feature assertions
- Redact registry URLs before logging to prevent credential leakage - Skip registry cache when custom source URLs are configured (stale cache fix) - Runtime-validate symphonyRegistryUrls from settings store - Reset modal-open flags when Encore Feature toggles are disabled - Normalize registry URLs before duplicate/default checks - Add aria-label to icon-only registry URL remove button - Expose setSymphonyRegistryUrls in getSettingsActions() - Validate persisted symphonyRegistryUrls with Array.isArray guard
…r, and Encore feature flag - Register maestroCue as an Encore Feature flag (EncoreFeatureFlags, DEFAULT_ENCORE_FEATURES) - Create src/main/cue/cue-types.ts with CueEventType, CueSubscription, CueSettings, CueConfig, CueEvent, CueRunStatus, CueRunResult, CueSessionStatus, and related constants - Add 'CUE' to HistoryEntryType across shared types, global.d.ts, preload, IPC handlers, and hooks - Add cueTriggerName, cueEventType, cueSourceSession optional fields to HistoryEntry - Add 'cue' log level to MainLogLevel, LOG_LEVEL_PRIORITY, logger switch/case, and LogViewer with teal color (#06b6d4), always-enabled filter, and agent name pill - Add 10 Cue-specific template variables (CUE_EVENT_TYPE, CUE_TRIGGER_NAME, etc.) with cueOnly flag - Extend TemplateContext with cue? field and substituteTemplateVariables with Cue replacements - Update TEMPLATE_VARIABLES_GENERAL filter to exclude cueOnly variables
…ovider Implements the three core modules for the Cue event-driven automation engine: - cue-yaml-loader.ts: Discovers and parses maestro-cue.yaml files with js-yaml, validates config structure, watches for file changes via chokidar with 1-second debounce - cue-file-watcher.ts: Wraps chokidar for file.changed subscriptions with per-file debouncing (5s default), constructs CueEvent instances with full file metadata payloads - cue-engine.ts: Main coordinator class with dependency injection, manages time.interval timers (fires immediately then on interval), file watchers, agent.completed listeners with fan-in tracking, activity log ring buffer (max 500), and run lifecycle management Added js-yaml and @types/js-yaml dependencies. 57 tests across 3 test files.
…story recording Implements the Cue executor module that spawns background agent processes when Cue triggers fire, following the same spawn pattern as Auto Run's process:spawn IPC handler. Key exports: - executeCuePrompt(): Full 10-step pipeline (prompt resolution, template substitution, agent arg building, SSH wrapping, process spawn with stdout/stderr capture, timeout enforcement with SIGTERM→SIGKILL) - stopCueRun(): Graceful process termination by runId - recordCueHistoryEntry(): Constructs HistoryEntry with type 'CUE' and all Cue-specific fields (trigger name, event type, source session) - getActiveProcesses(): Monitor running Cue processes Test coverage: 31 tests in cue-executor.test.ts covering execution paths, SSH remote, timeout escalation, history entry construction, and edge cases. Full suite: 21,635 tests passing across 512 files, zero regressions.
…zation for Maestro Cue
Add CUE entry support across all History components: - HistoryFilterToggle: CUE filter button with teal (#06b6d4) color and Zap icon - HistoryEntryItem: CUE pill, success/failure badges, and trigger metadata subtitle - HistoryPanel & UnifiedHistoryTab: CUE included in default activeFilters - HistoryDetailModal: CUE pill color, icon, success/failure indicator, trigger metadata display - Comprehensive test coverage for all CUE rendering paths (205 new/updated tests pass)
…nd activity log Add the Maestro Cue dashboard modal with full Encore Feature gating: - CueModal component with sessions table, active runs list, and activity log - useCue hook for state management, event subscriptions, and 10s polling - Settings toggle in Encore tab, command palette entry, keyboard shortcut (Cmd+Shift+U) - SessionList hamburger menu entry, modal store integration, lazy loading - 30 tests covering hook behavior and modal rendering
Add CueYamlEditor component for creating and editing maestro-cue.yaml files. Features split-view layout with AI assist (left panel for description + clipboard copy) and YAML editor (right panel with line numbers, debounced validation, Tab indentation). Integrates into CueModal via Edit YAML button on each session row.
…yaml Task 1: CueHelpModal component with 7 content sections (What is Maestro Cue, Getting Started, Event Types, Template Variables, Multi-Agent Orchestration, Timeouts & Failure Handling, AI YAML Editor). Wired to CueModal ? button. Registered with layer stack at MODAL_PRIORITIES.CUE_HELP (465). Task 2: useCueAutoDiscovery hook that calls cue:refreshSession when sessions are created/restored/removed, gated by encoreFeatures.maestroCue. Full scan on feature enable, engine disable on feature off. Tests: 38 CueHelpModal tests + 10 useCueAutoDiscovery tests, all passing. Lint clean. No existing test regressions (21,778 tests pass).
… session bridging Implement agent completion event chaining in the Cue engine: - Fan-out: subscriptions dispatch prompts to multiple target sessions simultaneously - Fan-in: subscriptions wait for all source sessions to complete before firing, with timeout handling (break clears tracker, continue fires with partial data) - Session bridging: user session completions trigger Cue subscriptions via exit listener - Add AgentCompletionData type for rich completion event payloads - Add hasCompletionSubscribers() optimization to skip unneeded notifications - Wire getCueEngine/isCueEnabled into ProcessListenerDependencies
…ure gated) Add teal Zap icon next to session names in the Left Bar for sessions with active Maestro Cue subscriptions. The indicator is gated behind the maestroCue Encore Feature flag and shows a tooltip with the subscription count on hover. - Add cueSubscriptionCount prop to SessionItem with Zap icon rendering - Add lightweight Cue status fetching in SessionListInner via cue:getStatus IPC, refreshed on cue:activityUpdate events - Add cue namespace to global test setup mock - 6 unit tests + 3 integration tests; all 21,815 tests pass; lint clean
Add Maestro Cue entries across all developer documentation: - CLAUDE.md: Key Files table (4 entries), Architecture tree (cue/ dir), Standardized Vernacular (Cue + Cue Modal terms) - CLAUDE-PATTERNS.md: Encore Feature section lists maestroCue as second reference implementation alongside directorNotes - CLAUDE-IPC.md: cue namespace in Automation section, full Cue API reference with all endpoints and event documentation
…t journal - Add cue-db.ts: SQLite-backed event journal (cue_events table) and single-row heartbeat table (cue_heartbeat) using better-sqlite3 with WAL mode - Add cue-reconciler.ts: time event catch-up logic that fires exactly one reconciliation event per missed subscription (no flooding), with payload.reconciled and payload.missedCount metadata - Update cue-engine.ts: heartbeat writer (30s interval), sleep detection (2-minute gap threshold), database pruning (7 days), and clean shutdown - Update CueHelpModal: new "Sleep & Recovery" section with Moon icon - Update CueModal: amber "catch-up" badge on reconciled activity log entries - Tests: 41 new tests across cue-db (17), cue-reconciler (11), cue-sleep-wake (13)
Add filter field to CueSubscription for narrowing when subscriptions fire. Supports exact match, negation (!), numeric comparison (>/</>=/<=), glob patterns (picomatch), and boolean matching with AND logic. Filter checks integrated at all three dispatch points (file.changed, time.interval, agent.completed). Includes help modal docs, AI prompt updates, and 80 new tests (43 filter engine + 37 YAML loader).
… awareness Add pattern presets (Scheduled Task, File Enrichment, Reactive, Research Swarm, Sequential Chain, Debate) to the YAML editor as clickable cards. Enhance the AI system prompt with pattern recognition guidance. Add a Coordination Patterns section with ASCII flow diagrams to the help modal.
Add github.pull_request and github.issue event types to CueEventType union. Add repo and poll_minutes fields to CueSubscription interface. Add cue_github_seen SQLite table with 5 CRUD functions for tracking seen GitHub items (isGitHubItemSeen, markGitHubItemSeen, hasAnyGitHubSeen, pruneGitHubSeen, clearGitHubSeenForSubscription). Create cue-github-poller.ts module that polls GitHub CLI for new PRs/issues, seeds existing items on first run, and fires CueEvents for new items. Comprehensive test suite with 17 test cases covering all polling behaviors. All 264 Cue tests pass, lint clean.
- Document Graph controls now auto-close other dropdowns for cleaner UX 🧭 - Session bookmarks hide automatically when filtering unread agents only 🔖 - Unread tab filter now keeps busy tabs visible for better awareness ⏳ - Tab keyboard shortcuts now work across AI, terminal, and file modes ⌨️ - Unread-only navigation now treats busy tabs as navigable targets 🧠
When showUnreadAgentsOnly is active, agent cycling (Cmd+[/]) now only visits unread or busy agents instead of all visible agents. The currently active agent is always included so you don't get stuck. Claude ID: 21220442-55d4-4a7c-ba6c-34d278946cf4 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
…ssions 🧩 - OS notifications are kept alive to prevent lost click events 🛡️ - Notification lifecycle cleanup now runs on close and click reliably 🧹 - Agent drawer auto-focuses the search box when opened for faster picking 🎯 - Tab Bar tooltips now reflect user-configured shortcut bindings dynamically ⌨️ - Unread-only session filtering preserves your active session visibility 🧭 - Unread filter now considers unread/busy worktree children, not just parents 🌿 - Worktrees auto-expand while filtering unread so nothing important is hidden 🔎 - Right panel focus ring clears correctly when focus leaves the panel 🎛️ - Focus ring styling no longer bumps z-index, reducing visual layering glitches 🪟
…olish - Tab naming: resolve early from partial output (2s interval) instead of waiting for full process exit; bump timeout 30s → 45s for cold starts - Cue pipeline: prefer subscription name match over stale agent_id when resolving target sessions; fall back to name-based lookup when agent_id is absent - Auto Run: return empty content with notFound flag for missing files instead of throwing, avoiding errors on deleted/renamed documents - LogViewer: add per-entry copy button with clipboard support - AutoRunDocumentSelector: use file icons from theme, move task percentage badge to right-aligned position - Web server: avoid returning stale logs from wrong tab during new tab creation race
Match the tab filter icon pattern where the accent dot is always visible rather than conditional on hasUnreadAgents. Removed the now-unused hasUnreadAgents prop and memo.
…Now for Cue pipelines - Edge-based prompts: each trigger→agent edge can carry its own prompt, enabling multiple triggers to feed the same agent with different instructions - Custom trigger labels: user-defined names (e.g. "Morning Check") displayed on trigger nodes and serialized as subscription labels in YAML - Run Now button: manually trigger any subscription from the Cue dashboard, bypassing event conditions via synthetic event dispatch - Visual polish: pulsing animation on active nodes, larger arrowheads on selected edges, disabled drawer buttons when viewing all pipelines - YAML round-trip: yamlToPipeline deserializes edge prompts and trigger labels; pipelineToYaml generates unique prompt file paths for multi-trigger agents - Tests: 4 new test cases covering edge prompts, custom labels, multi-trigger serialization, and unique prompt file path generation Claude ID: 3ce19e17-50c0-47ff-bc36-f979900ec700 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Defer splash dismissal until UI has painted with loaded data, preventing the unresponsive gap after the loading screen clears. Progress stages now follow an orchestra warm-up sequence: tuning instruments → reading the score → seating the musicians → warming up the ensemble → the concertmaster rises → maestro takes the podium → curtain up. Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Adds a chevron toggle in the drawer header that expands the node configuration panel from 200px to 80% viewport height. When expanded, textareas switch from fixed rows to flex-fill layout, responsively splitting available space among input/output prompts and per-edge trigger prompts. Includes smooth height transition animation. Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3 Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
Worktree and wizard sessions now start with only an AI tab. Users can launch terminal tabs on demand. Removes unnecessary createTerminalTab calls and imports from worktreeSession.ts and useWizardHandlers.ts. Claude ID: c57f388f-2e03-48f6-b055-bfb0510fd774 Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
The worktree children filter in SessionList was hiding the active session when it had no unread tabs and wasn't busy. Added activeSessionId exemption so the user's current worktree always appears in the left bar.
…pdate tests - Fix DEFAULT_SHORTCUTS.maestroCue → openCue in CueHelpModal (runtime crash) - Remove orphaned auto-scroll toggle JSX and state refs in TerminalOutput - Remove orphaned CueYamlEditor import/state in CueModal (replaced by pipeline editor) - Remove orphaned stats/WakaTime settings and effects in EncoreTab - Remove duplicate filterUnreadAgents branch in useMainKeyboardHandler - Add missing imports in DocumentGenerationView (MermaidRenderer, useClickOutside, etc.) - Prefix unused readOnlyMode param in agent-spawner - Remove unnecessary eslint-disable comment in MainPanel - Update CueModal test: YAML editor assertion → stub no-throw check - Update keyboard handler test: Cmd+T in terminal mode correctly blocked (AI mode only)
SSH Group Chat participants crashed immediately (exit code 1) because large prompts were embedded in bash -c CLI args via buildSshCommand(), where ProcessManager couldn't detect --input-format stream-json flags to enable stdin delivery. Switched to buildSshCommandWithStdin() for large prompts, matching the proven pattern in process:spawn IPC handler. Also added missing SSH wrapping to spawnModeratorSynthesis().
Added a useEffect in App.tsx that tracks inputMode transitions via a ref. When switching from terminal back to AI mode (via tab click, Cmd+J, or any other path), the input textarea is automatically focused so the user can immediately start typing. Claude ID: 3ce1f013-9673-40ec-ac0c-5cf1eaca0198 Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
- Session cleanup now kills AI, legacy terminal, and tab PTYs reliably 🔥 - Terminal tabs get proper per-tab process IDs via `getTerminalSessionId` 🧩 - Closing sessions now iterates all `terminalTabs` to terminate each PTY ✅ - Deleting sessions also shuts down every terminal tab process cleanly 🧹 - Improved test coverage for multi-tab PTY termination on close 🧪 - More resilient error handling around terminal-tab process kills 🛡️ - Sentry captures tab-kill failures with session and tab context 🕵️ - Clearer lifecycle comments and intent: “kill all processes” upfront 📝
Cmd+0 was previously consumed by font size reset, blocking the goToLastTab shortcut. Now Cmd+0 goes to the last tab (consistent with Cmd+1-9 tab navigation) and Cmd+Shift+0 resets font size. Font size reset is now customizable in Settings → Shortcuts (moved from FIXED_SHORTCUTS to DEFAULT_SHORTCUTS).
- Splash screen now waits for initial file tree readiness before dismissing 🎬 - Added `initialFileTreeReady` gate to session store initialization flow 🧱 - New splash progress stage highlights file indexing at 80% 🗂️ - UI rendering stage bumped to 90% once file tree finishes loading 📈 - File tree manager signals readiness exactly once on success or error 🔔 - Session restoration unblocks splash immediately when no sessions exist 🧹 - Session-load failures now mark file tree ready to prevent startup hangs 🧯 - App initialization reacts to settings, sessions, and file tree readiness 🔄 - Improved startup messaging: “Indexing the score...” during file discovery 🎻 - Updated tests to cover new three-gate splash behavior and progress ✅
The scroll-into-view logic used full container bounds but didn't subtract the widths of the sticky left (search/filter) and right (+) elements, causing the active tab's close button to be hidden behind them when jumping to a tab or after auto-rename.
Session rows and process rows now show a hover-visible ExternalLink button that navigates to the agent (session) or specific tab and closes the modal. Group chat processes get a similar button that navigates to the group chat.
Use module-level sshStore to apply SSH wrapping in spawnModeratorSynthesis, matching the pattern used by routeUserMessage. Adds 2 tests verifying SSH config is applied/skipped correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ug in group-chat-router (TASK-M09) Replaced all console.log/error/warn calls in group-chat-router.ts with structured logger.debug/warn/error calls using LOG_CONTEXT. Consolidated sequential debug logs into single calls with structured data objects. Key changes: - JSON.stringify(finalArgs) → argCount + agentType - Full prompt length logging → promptLengthChars field - Message preview strings → messageLength only - Removed decorative banner lines (========) - Removed unused spawnResult assignments (only used for logging) - console.error/warn → logger.error/warn with structured context Reduced from 130 console.* calls to 90 structured logger calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openasocket
force-pushed
the
fix/group-chat-quality
branch
from
0a3bc21 to
4e63ba6
Compare
Contributor
Author
|
Retargeted from |
npm ci was failing because package-lock.json was out of sync with package.json after rebasing. Missing: @types/js-yaml, @types/picomatch, picomatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spawnModeratorSynthesis()— synthesis was the only spawn point missing SSH support, causing moderator initial spawn to run remotely but synthesis to run locally (TASK-M04)console.logcalls in group-chat-router with structuredlogger.debug/warn/error— prevents sensitive data (args, session IDs) from bypassing log-level gating (TASK-M09)Cherry-picked from
feat/gemini-cliaudit fixes (TASK-M04, TASK-M09). Gemini-specific code (moderatorCwd, geminiNoSandbox, readOnlyCliEnforced) excluded — only shared infrastructure improvements.Test plan
console.logremaining in group-chat-router.ts🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores